Insert file mentions when non-image files are dropped on the composer - #5390
Insert file mentions when non-image files are dropped on the composer#53900xjohnnydev wants to merge 2 commits into
Conversation
Dropping a file onto the composer previously routed every file through the image-attachment path, so anything that wasn't an image was rejected with "Unsupported file type ... attach image files only". There was no way to drop a file from Finder/Explorer to reference it by path. Now an OS drop is partitioned: images still attach inline, and other files are turned into composer file mentions using the dropped File's on-disk path, resolved via Electron `webUtils.getPathForFile` exposed on the desktop bridge. Paths inside the workspace cwd are made workspace-relative so they match typed and file-tree mentions; paths outside stay absolute. Files whose path can't be resolved (browser builds, in-memory Files) fall back to the existing image path, so non-desktop behavior is unchanged. - contracts: add optional DesktopBridge.getPathForFile - desktop/preload: implement it via webUtils - web: partitionDroppedComposerFiles / toComposerMentionPath / buildDroppedFileMentions helper + unit tests, wired into ChatComposer onComposerDrop
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| export function toComposerMentionPath(absolutePath: string, cwd: string | null): string { | ||
| const normalizedPath = absolutePath.replace(/\\/g, "/"); | ||
| if (cwd !== null) { | ||
| const normalizedCwd = cwd.replace(/\\/g, "/").replace(/\/+$/, ""); |
There was a problem hiding this comment.
🟡 Medium chat/composerFileDrop.ts:39
toComposerMentionPath strips all trailing slashes from cwd, so when cwd is "/" the normalized cwd becomes an empty string and the relativization branch is skipped entirely. Dropping /src/app.ts with the workspace rooted at / returns /src/app.ts instead of the expected src/app.ts. Consider preserving POSIX root "/" as the prefix when normalizing instead of stripping it down to an empty string.
export function toComposerMentionPath(absolutePath: string, cwd: string | null): string {
const normalizedPath = absolutePath.replace(/\\/g, "/");
if (cwd !== null) {
- const normalizedCwd = cwd.replace(/\\/g, "/").replace(/\/+$/, "");
+ const normalizedCwd =
+ cwd.replace(/\\/g, "/") === "/"
+ ? "/"
+ : cwd.replace(/\\/g, "/").replace(/\/+$/, "");
if (normalizedCwd.length > 0) {🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/composerFileDrop.ts around lines 39-42:
`toComposerMentionPath` strips all trailing slashes from `cwd`, so when `cwd` is `"/"` the normalized cwd becomes an empty string and the relativization branch is skipped entirely. Dropping `/src/app.ts` with the workspace rooted at `/` returns `/src/app.ts` instead of the expected `src/app.ts`. Consider preserving POSIX root `"/"` as the prefix when normalizing instead of stripping it down to an empty string.
…ions Review fixes for the drop-to-mention handler: - The mention token grammar (FILE_LINK_TOKEN_REGEX) requires trailing whitespace after a token, and every other insert site appends one; without it the last dropped mention stayed plain text and was missed at send time. - Focusing synchronously after the prompt insert made focusAt push the editor's stale pre-mention snapshot back through onChange, overwriting the inserted mention -- the exact failure documented on ComposerMentionDropHost. The insert path already focuses on the next frame, so only focus here when no mention was inserted.
ApprovabilityVerdict: Needs human review This PR introduces new user-facing behavior by enabling non-image file drops to become file mentions in the composer. New features that change runtime behavior warrant human review. Additionally, there is an unresolved comment identifying a bug in path handling for POSIX root directories. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Problem
Dropping a file onto the composer routes every dropped file through the image-attachment path (
onComposerDrop→addComposerImages). Anything that isn't an image is rejected with:So there's no way to drag a file from Finder/Explorer to reference it by path — you can only do it by typing
@or dragging from the in-app file tree.Change
onComposerDropnow partitions the dropped files:File's on-disk path resolved via ElectronwebUtils.getPathForFile, exposed as a new optionalDesktopBridge.getPathForFile.Files) fall back to the existing image path, so the "unsupported file type" error still surfaces and non-desktop behavior is unchanged.Files
packages/contracts/src/ipc.ts— add optionalDesktopBridge.getPathForFile(typedunknownsince contracts is built without the DOM lib).apps/desktop/src/preload.ts— implement it viawebUtils.getPathForFile(synchronous, in-process, no IPC hop).apps/web/src/components/chat/composerFileDrop.ts—partitionDroppedComposerFiles,toComposerMentionPath,buildDroppedFileMentions(pure helpers) +composerFileDrop.test.ts.apps/web/src/components/chat/ChatComposer.tsx— wire helpers intoonComposerDrop;insertComposerTextAtEndmoved above the drop handler so it's defined before use.Notes / open questions
getPathForFileresolves paths on the local desktop host, while mentions are resolved against the environment workspace. On a local environment (the common case) these agree. On SSH/WSL environments the inserted mention would be a local path the backend can't read (before this PR such drops errored loudly). Options: gate mention insertion on the environment being local, or accept the dangling reference. Happy to implement whichever you prefer.Note
Insert file-mention tokens when non-image files are dropped on the composer
getPathForFile(file)method towindow.desktopBridge(via preload.ts and theDesktopBridgeinterface) that synchronously returns the absolute backing path for a DOMFile.onComposerDrophandler inChatComposerto resolve paths viadesktopBridge, insert mention tokens for resolved non-image files, and fall back to image attachment for unresolved files. An error toast is shown if the composer is busy when mentions need to be inserted.Macroscope summarized 254a259.
Note
Medium Risk
Composer drop behavior changes on desktop (mentions vs image errors), and resolved paths are host-local which may not match SSH/WSL workspace paths.
Overview
OS file drops on the chat composer no longer send every file through the image-attachment path. Dropped files are split into images (still attached inline) and everything else, which on desktop can become file mention tokens in the prompt.
A new optional
DesktopBridge.getPathForFile(contracts + Electron preload viawebUtils.getPathForFile) resolves the on-disk path for each non-imageFile.composerFileDrophelpers relativize in-workspace paths to match typed/file-tree mentions, serialize links withserializeComposerFileLink, and are covered by unit tests.ChatComposer.onComposerDropinserts those mentions (with error toast if the composer is busy), avoids eager focus after a successful mention insert, and routes path-unresolved files back throughaddComposerImagesso browser builds and in-memory files keep the prior “unsupported file type” behavior.Reviewed by Cursor Bugbot for commit 254a259. Bugbot is set up for automated code reviews on this repo. Configure here.